Conversation
| except (asyncio.TimeoutError, aiohttp.ClientError) as err: | ||
| try_again(err) | ||
| """Retry in at least 20 minutes.""" | ||
| minutes = 15 + randrange(5) |
There was a problem hiding this comment.
This is not 'at least 20 minutes'. This is '15 to 19 minutes'.
There was a problem hiding this comment.
Yes, I will update the comment
| hass, weather.async_update, minute=randrange(1, 10), | ||
| second=randrange(0, 59)) | ||
| yield from weather.async_update() | ||
| async_track_utc_time_change(hass, weather.async_update, second=0) |
There was a problem hiding this comment.
this is not on the hour. This is called every minute.
There was a problem hiding this comment.
Read the code, looks like just the comment is stale.
There was a problem hiding this comment.
So this is very inefficient. Instead of calling update every minute, we should just keep track when we want to fetch data, then when data is fetched, we should call the update method.
I think that it should be something like this:
def async_setup_platform(…):
async_call_later(hass, 5, weather.update_data)
class YrData(object):
@asyncio.coroutine
def update_data(self):
try:
yield from fetching_data()
yield from updating_devices()
async_call_later(hass, random(X), self.update_data)
except …:
async_call_later(hass, random(15), self.update_data)There was a problem hiding this comment.
Wait, it seems like you're doing this already. So this line can be removed
There was a problem hiding this comment.
We want to call updating_devices() more often and more regular than fetching_data(), but every minute is probably overkill.
When we fetch the data we get the forecast for several hours:
Eg: 10:00, 11:00, 12:00, 13:00, 14:00,
So at 10:31 we want to show the 11:00 forecast as that is closest in time.
We still want to fetch new data each hour, to make sure we have the newest and best forecast.
There was a problem hiding this comment.
Ah, I see. In a perfect world we would set a timer for exactly that time to push an update.
| async_track_utc_time_change(hass, weather.updating_devices, minute=31) | ||
| # wait 1 minute after start up before we fetch data, | ||
| # to avoid several restarts to fetch new data. | ||
| async_call_later(hass, 60, weather.fetching_data) |
There was a problem hiding this comment.
Do we want this?
This show the weather info as unknown for the first minute.
It will reduce the load on their servers, but the user experience will be lower?
There was a problem hiding this comment.
This also makes the tests fail
There was a problem hiding this comment.
You can also just call it directly. The impact on their servers should be minimal as this is a one time call
There was a problem hiding this comment.
how about using the last value in recorder ? (like input_*)
If someone is testing/configuring and keeps restarting, he will keep hammering the endpoint
* Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * fix comment * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Spread the load more for the yr.no sensor * Update yr.py
ghost
removed
the
5 participants
Description:
Reduce the load on met.no servers, yr.no sensor
Try to spread the load more.
Each user will at initialization of the sensor get a random time (min:sec), and will only ask for new data at that time
Related issue (if applicable): fixes #12425
Example entry for
configuration.yaml(if applicable):Checklist: